-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Change large_futures such that it applies to all expressions of type Future #16349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
No changes for 93216cf |
343d882 to
207a248
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Not sure this will fix your problem completely, but it looks like at least the annotations for async blocks are incorrect. You write them as follows: async {
let x = [0i32; 1024 * 16];
async {}.await;
dbg!(x);
}
//~^ large_futuresBut the diagnostic is emitted at the first line of a multiline object, so it should look like this: async {
let x = [0i32; 1024 * 16];
async {}.await;
dbg!(x);
}
//~^^^^^ large_futures(the number of repeated or like this: //~v large_futures
async {
let x = [0i32; 1024 * 16];
async {}.await;
dbg!(x);
} |
Thank you, that is indeed what I did not comprehend yet. The underlying issue is that I am (apparantly) emitting duplicate lints for the same spans. When running clippy, these seem to be deduplicated. For the tests however this does not happen. I am unsure though why my visitors seem to visit the same expressions multiple times. What would be the proper way to fix this? Keep a BTreeSet of the spans around and duplicate them myself? |
Deduplication is disabled in tests, yes. The test suite did emit a note about this: But it was drowned in the see of other diagnostics, so I understand how it could be overlooked^^
I think this is because
I think |
Ok this error message is a bit cryptic as well I imagine... The reason is that you only provide an autofix for async expressions, but not functions -- so after the autofixes are applied, the warnings on async functions are still emitted. This is fine -- the way you handle this is as follows:
|
6ce6907 to
34f1957
Compare
changelog: [large_futures]: changed lint such that it applies to all expressions of type Future.
Currently large_futures is only applicable to expressions on
awaits that are a call to a function of which the type is larger than a given threshold.This means that the lint does not apply if in the code
awaitis never called on the future. Examples of this include runningblock_onor similar functions, or if the future is passed to an executor. Also it is possible to create futures that are not explicit functions, likeasync {}blocks, or types thatimpl Futuredirectly.This PR attempts to make this lint applicable to all expressions of the type Future. It will try to find the 'deepest' expression in a branch of the expression tree for which this holds, and does not emit a message for any shallower elements in that branch.
Note that this PR currently fails testing as the UI toolkit seems to be very confused about the emitted spans. Multiple messages seem to be emitted for the same span, and the diagnostics do not seem to match at all. Any help concerning this is very appreciated. Example:
Relevant code: